Avoid unused clones in `Cloned` and `Copied` #139745

thaliaarchi · 2025-04-13T11:37:52Z

Avoid cloning in Cloned or copying in Copied when elements are only needed by reference or not at all. There is already some precedent for this, given that __iterator_get_unchecked is implemented, which can skip elements. The reduced clones are technically observable by a user impl of Clone.

r? libs-api

thaliaarchi · 2025-04-13T11:43:47Z

I was trying to implement Iterator::try_find for them, but I had difficulty figuring out how to map the residual. Perhaps this is missing from the try_trait_v2 API? Or, more likely, I'm missing something. I'd appreciate help on this.

joboet · 2025-04-13T16:40:42Z

I was trying to implement Iterator::try_find for them, but I had difficulty figuring out how to map the residual. Perhaps this is missing from the try_trait_v2 API? Or, more likely, I'm missing something. I'd appreciate help on this.

The easiest way is to use try-blocks:

try { it.try_find(..)?.cloned() }

thaliaarchi · 2025-04-13T23:21:43Z

Thanks. That's indeed simpler. Unfortunately, it's not the full missing piece to the puzzle.

With that, it's this:

impl<'a, I, T: 'a> Iterator for Cloned<I>
where
    I: Iterator<Item = &'a T>,
    T: Clone,
{
    fn try_find<R>(
        &mut self,
        f: impl FnMut(&Self::Item) -> R,
    ) -> ChangeOutputType<R, Option<Self::Item>>
    where
        R: Try<Output = bool, Residual: Residual<Option<Self::Item>>>,
    {
        try { self.it.try_find(move |x| f(&x))?.cloned() }
    }
}

It yields this error:

error[E0277]: the trait bound `<R as Try>::Residual: Residual<Option<&'a T>>` is not satisfied
    --> library/core/src/iter/adapters/cloned.rs:97:23
     |
97   |         try { self.it.try_find(move |x| f(&x))?.cloned() }
     |                       ^^^^^^^^ the trait `Residual<Option<&'a T>>` is not implemented for `<R as Try>::Residual`
     |
note: required by a bound in `iterator::Iterator::try_find`
    --> library/core/src/iter/traits/iterator.rs:2930:41
     |
2924 |     fn try_find<R>(
     |        -------- required by a bound in this associated function
...
2930 |         R: Try<Output = bool, Residual: Residual<Option<Self::Item>>>,
     |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Iterator::try_find`

This is the same error I was encountering with my draft version. I think it is equivalent or close to using try blocks:

match self.it.try_find(move |x| f(&x)).branch() {
    ControlFlow::Continue(x) => Try::from_output(x),
    ControlFlow::Break(r) => FromResidual::from_residual(r),
}

Upon further reflection, I think try_find is incompatible with this PR. As I understand the Residual<T> bound, the T is the type of the output and the type of the residual is unconstrained. Since it has access to the T of the output, the residual could be derived from it. As the iterator items differ, residuals which use that type will differ and cannot be converted between each other.

Avoid cloning in `Cloned` or copying in `Copied` when elements are only needed by reference or not at all. There is already some precedent for this, given that `__iterator_get_unchecked` is implemented, which can skip elements. The reduced clones are technically observable by a user impl of `Clone`.

joboet · 2025-04-14T10:58:06Z

Ah, yes, that's true...

I don't think this needs libs-api approval, I'd consider these changes as implementation details. So:
@bors r+ rollup
r? joboet

bors · 2025-04-14T10:58:09Z

📌 Commit ed5f31a has been approved by joboet

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139745 (Avoid unused clones in `Cloned` and `Copied`) - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata) - rust-lang#139778 (Add test for issue 34834) - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests) - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it) - rust-lang#139799 (Specify `--print info=file` syntax in `--help`) - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically) - rust-lang#139813 (Miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup

…, r=joboet Avoid unused clones in `Cloned` and `Copied` Avoid cloning in `Cloned` or copying in `Copied` when elements are only needed by reference or not at all. There is already some precedent for this, given that `__iterator_get_unchecked` is implemented, which can skip elements. The reduced clones are technically observable by a user impl of `Clone`. r? libs-api

Rollup of 12 pull requests Successful merges: - rust-lang#138374 (Enable contracts for const functions) - rust-lang#138380 (ci: add runners for vanilla LLVM 20) - rust-lang#138393 (Allow const patterns of matches to contain pattern types) - rust-lang#139393 (rustdoc-json: Output target feature information) - rust-lang#139517 (std: sys: process: uefi: Use NULL stdin by default) - rust-lang#139554 (std: add Output::exit_ok) - rust-lang#139745 (Avoid unused clones in `Cloned` and `Copied`) - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata) - rust-lang#139778 (Add test for issue 34834) - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests) - rust-lang#139789 (do not unnecessarily leak auto traits in item bounds) - rust-lang#139791 (drop global where-bounds before merging candidates) r? `@ghost` `@rustbot` modify labels: rollup

…, r=joboet Avoid unused clones in `Cloned` and `Copied` Avoid cloning in `Cloned` or copying in `Copied` when elements are only needed by reference or not at all. There is already some precedent for this, given that `__iterator_get_unchecked` is implemented, which can skip elements. The reduced clones are technically observable by a user impl of `Clone`. r? libs-api

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139745 (Avoid unused clones in `Cloned` and `Copied`) - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata) - rust-lang#139778 (Add test for issue 34834) - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests) - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it) - rust-lang#139799 (Specify `--print info=file` syntax in `--help`) - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically) - rust-lang#139813 (Miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139745 - thaliaarchi:iter-unused-clone-copy, r=joboet Avoid unused clones in `Cloned` and `Copied` Avoid cloning in `Cloned` or copying in `Copied` when elements are only needed by reference or not at all. There is already some precedent for this, given that `__iterator_get_unchecked` is implemented, which can skip elements. The reduced clones are technically observable by a user impl of `Clone`. r? libs-api

…, r=joboet Avoid unused clones in `Cloned` and `Copied` Avoid cloning in `Cloned` or copying in `Copied` when elements are only needed by reference or not at all. There is already some precedent for this, given that `__iterator_get_unchecked` is implemented, which can skip elements. The reduced clones are technically observable by a user impl of `Clone`. r? libs-api

rustbot assigned Amanieu Apr 13, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 13, 2025

thaliaarchi force-pushed the iter-unused-clone-copy branch from 8a6d677 to ed5f31a Compare April 13, 2025 23:23

rustbot assigned joboet Apr 14, 2025

rustbot unassigned Amanieu Apr 14, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2025

matthiaskrgr mentioned this pull request Apr 14, 2025

Rollup of 8 pull requests #139826

Merged

Zalathar mentioned this pull request Apr 15, 2025

Rollup of 12 pull requests #139837

Closed

jieyouxu mentioned this pull request Apr 15, 2025

Rollup of 11 pull requests #139842

Closed

bors merged commit efca25f into rust-lang:master Apr 15, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 15, 2025

thaliaarchi deleted the iter-unused-clone-copy branch April 15, 2025 08:20

DaniPopes mentioned this pull request Apr 23, 2025

copied().min() performance regression on nightly #140207

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid unused clones in `Cloned<I>` and `Copied<I>` #139745

Avoid unused clones in `Cloned<I>` and `Copied<I>` #139745

thaliaarchi commented Apr 13, 2025

thaliaarchi commented Apr 13, 2025

joboet commented Apr 13, 2025

thaliaarchi commented Apr 13, 2025

joboet commented Apr 14, 2025

bors commented Apr 14, 2025

Avoid unused clones in Cloned<I> and Copied<I> #139745

Avoid unused clones in Cloned<I> and Copied<I> #139745

Conversation

thaliaarchi commented Apr 13, 2025

thaliaarchi commented Apr 13, 2025

joboet commented Apr 13, 2025

thaliaarchi commented Apr 13, 2025

joboet commented Apr 14, 2025

bors commented Apr 14, 2025

Avoid unused clones in `Cloned<I>` and `Copied<I>` #139745

Avoid unused clones in `Cloned<I>` and `Copied<I>` #139745